Skip to content

event delegation does not work#4

Open
neonstalwart wants to merge 1 commit into
Raynos:masterfrom
neonstalwart:delegation
Open

event delegation does not work#4
neonstalwart wants to merge 1 commit into
Raynos:masterfrom
neonstalwart:delegation

Conversation

@neonstalwart

Copy link
Copy Markdown
Collaborator

this adds some failing tests for event delegation. i think i may have some
other tests to add too but just wanted to use these to start the conversation
about how delegation should work.

some of the other things i want to test are:

  • stopPropagation prevents delegation as expected
  • when global handlers are called in relation to other handlers (before, after)

let me know if delegation is supposed to work some other way so that i can get
it working in my project and try to add some other tests for that way.

Comment thread test/dom-delegation.js

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe - you could make the first test pass without making this test pass if you only change half of the condition at https://github.com/Raynos/dom-delegator/blob/v9.0.0/dom-delegator.js#L120

@Raynos

Raynos commented Jul 1, 2014

Copy link
Copy Markdown
Owner

I think the term your looking for is

event bubbling doesn't work

  • event bubbling is not implemented
  • stopImmediatePropagation is not implemented

Currently the events default to calling stopPropagation() automatically because that's a sensible
default. Most of the time you don't want it to bubble.

I've not had any need for either of those features so far.

I'd be interested in seeing what the use case is.

@neonstalwart

Copy link
Copy Markdown
Collaborator Author

the main use case i have right now is closing a modal dialog via ESC. if a component in a dialog is focused, when the user presses ESC, the dialog should close. currently, if any of the focused components have a keydown or an event handler already (which a lot of them do) then they prevent the bubbling.

another use case is navigating items in a list - when an item in the list is focused and the user presses a key to navigate to another item, the focus should move.

event delegation relies on event bubbling - so call it whatever you like, but it's not working 😦

Currently the events default to calling stopPropagation() automatically because that's a sensible
default. Most of the time you don't want it to bubble.

that's a really bad generalization to make. it's hard to say "most of the times" at all but if you said it for anything, i think i might say i do want it to always bubble but most of the times i call preventDefault to stop the default action.

at first i thought that there was simply no bubbling implemented in dom-delegator but then i saw that there were certain conditions where there's an attempt at simulating it https://github.com/Raynos/dom-delegator/blob/v9.0.0/dom-delegator.js#L121. this leads to inconsistencies that depend on what content i add as children to the parentNode. if i add content with no handlers then the events will bubble but if i add content with handlers i'll stop receiving the events. this is not good since neither the content nor the container should need to understand each other.

@Raynos

Raynos commented Jul 1, 2014

Copy link
Copy Markdown
Owner

@neonstalwart

I will implement stopPropagation() and startPropagation() on the event in dom-delegator

There will be a flag passed to DomDelegator({ ... }) that is options.bubbles which defaults to false.

If it's set to false then you must call startPropagation() to manually bubble.

If it's set to true then you must call stopPropagation() to manually stop bubbling.

I will update value-event to call the correct method at the correct time.

In mercury.app, the opts is already passed to the Delegator ( https://github.com/Raynos/mercury/blob/master/index.js#L52 ) so you can just set bubbles to true there

@neonstalwart

Copy link
Copy Markdown
Collaborator Author

this sounds like it would get me what i need. when it's ready, i'll try it out.

@Raynos

Raynos commented Aug 31, 2014

Copy link
Copy Markdown
Owner

@neonstalwart I just added the ability to .startPropagation() to v9.1.0

This means you can manually call .startPropagation() which is the inverse of the DOM .stopPropagation() and this will cause your event to bubble.

Whether bubbling or not should be the default is a seperate issue.

@Raynos

Raynos commented Aug 31, 2014

Copy link
Copy Markdown
Owner

I have not implemented opts.bubbles for DomDelegator nor have I implemented stopPropagation().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants